Skip to content

[AE-782] Build dap collector job for incrementality experiments #387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 52 commits into
base: main
Choose a base branch
from
Open

[AE-782] Build dap collector job for incrementality experiments #387

wants to merge 52 commits into from

Conversation

mashalifshin
Copy link

@mashalifshin mashalifshin commented Aug 18, 2025 •
edited
Loading

Problem Statement:

Currently, we are running incrementality experiments on FFHNT, measuring whether our Tiles are effective by seeing if they give a lift to organic visits, or to checkouts, to the tile advertiser. We're doing this in a private, anonymized manner, by sending the experiment results to DAP. However, we have no way to collect those results from DAP in an automated way and bring them into our usual data pipeline, so those results are currently being manually collected by an engineer.

Solution:

Create a docker-etl job that can go out to Nimbus and find out how to collect the results for each experiment branch, go out and collect those results from DAP, and finally write those results into BQ.

Checklist for reviewer:

  • Commits should reference a bug or github issue, if relevant (if a bug is
    referenced, the pull request should include the bug number in the title)
  • Scan the PR and verify that no changes (particularly to
    .circleci/config.yml) will cause environment variables (particularly
    credentials) to be exposed in test logs
  • Ensure the container image will be using permissions granted to
    telemetry-airflow
    responsibly.

gleonard-m
gleonard-m reviewed Aug 20, 2025
View reviewed changes
mashalifshin
mashalifshin commented Aug 21, 2025
View reviewed changes
@mashalifshin mashalifshin force-pushed the ae-782-build-dap-collector-job branch from ba47bdf to 65ef418 Compare August 29, 2025 00:53
@mashalifshin mashalifshin marked this pull request as ready for review August 30, 2025 02:22
@mashalifshin mashalifshin force-pushed the ae-782-build-dap-collector-job branch from 6c6a067 to 50591b2 Compare September 3, 2025 01:13
gleonard-m
gleonard-m reviewed Sep 3, 2025
View reviewed changes
@mashalifshin mashalifshin force-pushed the ae-782-build-dap-collector-job branch from 2f54368 to 63053fc Compare September 12, 2025 01:15
@mashalifshin mashalifshin force-pushed the ae-782-build-dap-collector-job branch from 38a4e04 to 8cb20c8 Compare September 25, 2025 23:09
gleonard-m
gleonard-m reviewed Oct 3, 2025
View reviewed changes
gleonard-m
gleonard-m reviewed Oct 3, 2025
View reviewed changes
gleonard-m
gleonard-m reviewed Oct 3, 2025
View reviewed changes
gleonard-m
gleonard-m reviewed Oct 3, 2025
View reviewed changes
gleonard-m
gleonard-m reviewed Oct 3, 2025
View reviewed changes
gleonard-m
gleonard-m reviewed Oct 3, 2025
View reviewed changes
gleonard-m
gleonard-m reviewed Oct 3, 2025
View reviewed changes
@mashalifshin mashalifshin force-pushed the ae-782-build-dap-collector-job branch from 80f87f4 to cbd7aca Compare October 8, 2025 00:13
mashalifshin and others added 18 commits October 7, 2025 17:13
@mashalifshin
Give batch duration a default of 7 days and make it optional in config
897e954
@mashalifshin
Add README and tests for default duration
82baa89
@mashalifshin
Lint and format (and re-enable logging to bucket that was accidentall…
d021855 
…y committed)
@mashalifshin
Fix bug where created_at was being written to be BQ as a datetime and…
3921ba5 
… not string
@mashalifshin
Fix up exception handling in main loop for throwing to airflow and up…
aabeecb 
…loading logs
@mashalifshin
Remove some unnecessary models
7318300
@mashalifshin
Expect created_at to be a string in insert_rows_json calls
3deecbd
@mashalifshin
Date mocking attempt 1: from unittest.mock docs, partial mocking section
a5e7952
@mashalifshin
Date mocking attempt 2: define a wrapper function on the class and mo…
b7bf0c9 
…ck that
@mashalifshin
Date mocking attempt 3: classmethod on MockDate wrapper class
0de2ef2
@mashalifshin
Date mocking attempt 4: Frozen mock class
becd5cd
@mashalifshin @chrisgramberg @luc-lisi
Date mocking success: Patch a wrapper function on experiment class in…
4830ea2 
… test_mocks, where experiment is instantiated

Co-authored-by: Chris Gramberg <[email protected]>
Co-authored-by: Luc Lisi <[email protected]>
@mashalifshin
Remove unnecessary DAP docs and generalize examples
692043e
@mashalifshin
Clean up done todo and use example value in test mocks
2aeb9ed
@mashalifshin
Separate params for config bucket gcp project and BQ table gcp project
164cfbf
@mashalifshin
Rename to created_timestamp, more example experiment names in mocks, …
cd40aca 
…and fix edge case in batch interval logic
@mashalifshin
Fix bugs in batch interval logic, TDD style
cbd7aca
@mashalifshin
Lint and fix up batch interval comments logic
c28d418
gleonard-m
gleonard-m reviewed Oct 8, 2025
View reviewed changes
jobs/ads-incrementality-dap-collector/README.md Outdated
Comment on lines 5 to 12
Incrementality is a way to measure the effectiveness of our ads in a general, agreggated, privacy-preserving way --
without knowing anything about specific users.

Incrementality works by dividing clients into various Nimbus experiment branches that vary how/whether an ad is shown.
Separately, a [DAP](https://docs.divviup.org/) task is configured to store the metrics for each experiment branch in a
different DAP bucket.

Firefox is instrumented with [DAP telemetry functionality](https://github.com/mozilla-firefox/firefox/tree/main/toolkit/components/telemetry/dap), which allows it to send metrics and reports into the correct DAP buckets as configured in the experiment.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the details of the experiment configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gleonard-m gleonard-m gleonard-m approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mashalifshin @gleonard-m